From 0ae2b97fcf56d665c9160f5b5cceefeba9dab162 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 9 Jul 2017 15:57:25 +0300 Subject: [PATCH] Warn about obsolete conventions for binary names --- src/cargo/util/paths.rs | 6 +- src/cargo/util/toml/targets.rs | 112 ++++++++++++++------------------- tests/build.rs | 93 ++++++++++++++++++++++++--- tests/git.rs | 23 ++----- tests/path.rs | 40 +++--------- tests/test.rs | 12 ++-- 6 files changed, 152 insertions(+), 134 deletions(-) diff --git a/src/cargo/util/paths.rs b/src/cargo/util/paths.rs index 988c4b9b4..ea6a66958 100644 --- a/src/cargo/util/paths.rs +++ b/src/cargo/util/paths.rs @@ -54,9 +54,9 @@ pub fn normalize_path(path: &Path) -> PathBuf { ret } -pub fn without_prefix<'a>(a: &'a Path, b: &'a Path) -> Option<&'a Path> { - let mut a = a.components(); - let mut b = b.components(); +pub fn without_prefix<'a>(long_path: &'a Path, prefix: &'a Path) -> Option<&'a Path> { + let mut a = long_path.components(); + let mut b = prefix.components(); loop { match b.next() { Some(y) => match a.next() { diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 0ee3746a6..9c176f8e8 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -16,6 +16,7 @@ use std::collections::HashSet; use core::Target; use ops::is_bad_artifact_name; use util::errors::CargoResult; +use util::paths::without_prefix; use super::{TomlTarget, LibKind, PathValue, TomlManifest, StringOrBool, TomlLibTarget, TomlBinTarget, TomlBenchTarget, TomlExampleTarget, TomlTestTarget}; @@ -38,7 +39,7 @@ pub fn targets(manifest: &TomlManifest, } targets.extend( - clean_bins(manifest.bin.as_ref(), package_root, package_name, has_lib)? + clean_bins(manifest.bin.as_ref(), package_root, package_name, warnings, has_lib)? ); targets.extend( @@ -233,6 +234,7 @@ fn clean_lib(toml_lib: Option<&TomlLibTarget>, fn clean_bins(toml_bins: Option<&Vec>, package_root: &Path, package_name: &str, + warnings: &mut Vec, has_lib: bool) -> CargoResult> { let inferred = inferred_bins(package_root, package_name); let bins = match toml_bins { @@ -259,17 +261,55 @@ fn clean_bins(toml_bins: Option<&Vec>, let mut result = Vec::new(); for bin in bins.iter() { - let path = bin.path.clone().unwrap_or_else(|| { - PathValue(inferred_bin_path(bin, has_lib, package_root, bins.len())) - }); - let mut target = Target::bin_target(&bin.name(), package_root.join(&path.0), + let path = match target_path(bin, &inferred, "bin", package_root) { + Ok(path) => path, + Err(e) => { + if let Some(legacy_path) = legacy_bin_path(package_root, &bin.name(), has_lib) { + { + let short_path = without_prefix(&legacy_path, package_root) + .unwrap_or(&legacy_path); + + warnings.push(format!( + "path `{}` was erroneously implicitly accepted for binary {},\n\ + please set bin.path in Cargo.toml", + short_path.display(), bin.name() + )); + } + legacy_path + } else { + return Err(e); + } + } + }; + + let mut target = Target::bin_target(&bin.name(), path, bin.required_features.clone()); configure(bin, &mut target); result.push(target); } - Ok(result) + return Ok(result); + + fn legacy_bin_path(package_root: &Path, name: &str, has_lib: bool) -> Option { + if !has_lib { + let path = package_root.join(format!("src/{}.rs", name)); + if path.exists() { + return Some(path); + } + } + let path = package_root.join("src/main.rs"); + if path.exists() { + return Some(path); + } + + let path = package_root.join("src/bin/main.rs"); + if path.exists() { + return Some(path); + } + return None; + } } + fn clean_examples(toml_examples: Option<&Vec>, package_root: &Path) -> CargoResult> { @@ -392,66 +432,6 @@ fn configure(toml: &TomlTarget, target: &mut Target) { }); } -fn inferred_bin_path(bin: &TomlBinTarget, - has_lib: bool, - package_root: &Path, - bin_len: usize) -> PathBuf { - // here we have a single bin, so it may be located in src/main.rs, src/foo.rs, - // src/bin/foo.rs, src/bin/foo/main.rs or src/bin/main.rs - if bin_len == 1 { - let path = Path::new("src").join("main.rs"); - if package_root.join(&path).exists() { - return path.to_path_buf(); - } - - if !has_lib { - let path = Path::new("src").join(&format!("{}.rs", bin.name())); - if package_root.join(&path).exists() { - return path.to_path_buf(); - } - } - - let path = Path::new("src").join("bin").join(&format!("{}.rs", bin.name())); - if package_root.join(&path).exists() { - return path.to_path_buf(); - } - - // check for the case where src/bin/foo/main.rs is present - let path = Path::new("src").join("bin").join(bin.name()).join("main.rs"); - if package_root.join(&path).exists() { - return path.to_path_buf(); - } - - return Path::new("src").join("bin").join("main.rs").to_path_buf(); - } - - // bin_len > 1 - let path = Path::new("src").join("bin").join(&format!("{}.rs", bin.name())); - if package_root.join(&path).exists() { - return path.to_path_buf(); - } - - // we can also have src/bin/foo/main.rs, but the former one is preferred - let path = Path::new("src").join("bin").join(bin.name()).join("main.rs"); - if package_root.join(&path).exists() { - return path.to_path_buf(); - } - - if !has_lib { - let path = Path::new("src").join(&format!("{}.rs", bin.name())); - if package_root.join(&path).exists() { - return path.to_path_buf(); - } - } - - let path = Path::new("src").join("bin").join("main.rs"); - if package_root.join(&path).exists() { - return path.to_path_buf(); - } - - return Path::new("src").join("main.rs").to_path_buf(); -} - fn target_path(target: &TomlTarget, inferred: &[(String, PathBuf)], target_kind: &str, diff --git a/tests/build.rs b/tests/build.rs index 9dfbbe7c3..05db15e75 100644 --- a/tests/build.rs +++ b/tests/build.rs @@ -540,12 +540,8 @@ fn cargo_compile_with_nested_deps_shorthand() { [dependencies.bar] path = "bar" - - [[bin]] - - name = "foo" "#) - .file("src/foo.rs", + .file("src/main.rs", &main_file(r#""{}", bar::gimme()"#, &["bar"])) .file("bar/Cargo.toml", r#" [project] @@ -687,7 +683,7 @@ fn cargo_compile_with_dep_name_mismatch() { path = "bar" "#) - .file("src/foo.rs", &main_file(r#""i am foo""#, &["bar"])) + .file("src/bin/foo.rs", &main_file(r#""i am foo""#, &["bar"])) .file("bar/Cargo.toml", &basic_bin_manifest("bar")) .file("bar/src/bar.rs", &main_file(r#""i am bar""#, &[])); @@ -1009,7 +1005,7 @@ fn many_crate_types_old_style_lib_location() { pub fn foo() {} "#); assert_that(p.cargo_process("build"), execs().with_status(0).with_stderr_contains("\ -[WARNING] path `src/foo.rs` was erroneously implicitly accepted for library foo, +[WARNING] path `src[/]foo.rs` was erroneously implicitly accepted for library foo, please rename the file to `src/lib.rs` or set lib.path in Cargo.toml")); assert_that(&p.root().join("target/debug/libfoo.rlib"), existing_file()); @@ -1326,6 +1322,81 @@ Caused by: can't find `hello` example, specify example.path")); } +#[test] +fn non_existing_binary() { + let mut p = project("world"); + p = p.file("Cargo.toml", r#" + [package] + name = "world" + version = "1.0.0" + authors = [] + + [[bin]] + name = "hello" + "#) + .file("src/lib.rs", "") + .file("src/bin/ehlo.rs", ""); + + assert_that(p.cargo_process("build").arg("-v"), execs().with_status(101).with_stderr("\ +[ERROR] failed to parse manifest at `[..]` + +Caused by: + can't find `hello` bin, specify bin.path")); +} + +#[test] +fn legacy_binary_paths_warinigs() { + let mut p = project("world"); + p = p.file("Cargo.toml", r#" + [package] + name = "foo" + version = "1.0.0" + authors = [] + + [[bin]] + name = "bar" + "#) + .file("src/lib.rs", "") + .file("src/main.rs", "fn main() {}"); + + assert_that(p.cargo_process("build").arg("-v"), execs().with_status(0).with_stderr_contains("\ +[WARNING] path `src[/]main.rs` was erroneously implicitly accepted for binary bar, +please set bin.path in Cargo.toml")); + + let mut p = project("world"); + p = p.file("Cargo.toml", r#" + [package] + name = "foo" + version = "1.0.0" + authors = [] + + [[bin]] + name = "bar" + "#) + .file("src/lib.rs", "") + .file("src/bin/main.rs", "fn main() {}"); + + assert_that(p.cargo_process("build").arg("-v"), execs().with_status(0).with_stderr_contains("\ +[WARNING] path `src[/]bin[/]main.rs` was erroneously implicitly accepted for binary bar, +please set bin.path in Cargo.toml")); + + let mut p = project("world"); + p = p.file("Cargo.toml", r#" + [package] + name = "foo" + version = "1.0.0" + authors = [] + + [[bin]] + name = "bar" + "#) + .file("src/bar.rs", "fn main() {}"); + + assert_that(p.cargo_process("build").arg("-v"), execs().with_status(0).with_stderr_contains("\ +[WARNING] path `src[/]bar.rs` was erroneously implicitly accepted for binary bar, +please set bin.path in Cargo.toml")); +} + #[test] fn implicit_examples() { let mut p = project("world"); @@ -2437,7 +2508,7 @@ fn invalid_spec() { [[bin]] name = "foo" "#) - .file("src/foo.rs", &main_file(r#""i am foo""#, &[])) + .file("src/bin/foo.rs", &main_file(r#""i am foo""#, &[])) .file("d1/Cargo.toml", r#" [package] name = "d1" @@ -3255,7 +3326,11 @@ fn no_bin_in_src_with_lib() { assert_that(p.cargo_process("build"), execs().with_status(101) - .with_stderr_contains(r#"[ERROR] couldn't read "[..]main.rs"[..]"#)); + .with_stderr_contains("\ +[ERROR] failed to parse manifest at `[..]` + +Caused by: + can't find `foo` bin, specify bin.path")); } diff --git a/tests/git.rs b/tests/git.rs index c6fae519d..5a9fda4a1 100644 --- a/tests/git.rs +++ b/tests/git.rs @@ -47,12 +47,8 @@ fn cargo_compile_simple_git_dep() { [dependencies.dep1] git = '{}' - - [[bin]] - - name = "foo" "#, git_project.url())) - .file("src/foo.rs", &main_file(r#""{}", dep1::hello()"#, &["dep1"])); + .file("src/main.rs", &main_file(r#""{}", dep1::hello()"#, &["dep1"])); let root = project.root(); let git_root = git_project.root(); @@ -116,11 +112,8 @@ fn cargo_compile_git_dep_branch() { git = '{}' branch = "branchy" - [[bin]] - - name = "foo" "#, git_project.url())) - .file("src/foo.rs", &main_file(r#""{}", dep1::hello()"#, &["dep1"])); + .file("src/main.rs", &main_file(r#""{}", dep1::hello()"#, &["dep1"])); let root = project.root(); let git_root = git_project.root(); @@ -186,12 +179,8 @@ fn cargo_compile_git_dep_tag() { git = '{}' tag = "v0.1.0" - - [[bin]] - - name = "foo" "#, git_project.url())) - .file("src/foo.rs", &main_file(r#""{}", dep1::hello()"#, &["dep1"])); + .file("src/main.rs", &main_file(r#""{}", dep1::hello()"#, &["dep1"])); let root = project.root(); let git_root = git_project.root(); @@ -552,12 +541,8 @@ fn recompilation() { version = "0.5.0" git = '{}' - - [[bin]] - - name = "foo" "#, git_project.url())) - .file("src/foo.rs", + .file("src/main.rs", &main_file(r#""{:?}", bar::bar()"#, &["bar"])); // First time around we should compile both foo and bar diff --git a/tests/path.rs b/tests/path.rs index bca1707fe..58eb43e88 100644 --- a/tests/path.rs +++ b/tests/path.rs @@ -28,12 +28,8 @@ fn cargo_compile_with_nested_deps_shorthand() { version = "0.5.0" path = "bar" - - [[bin]] - - name = "foo" "#) - .file("src/foo.rs", + .file("src/main.rs", &main_file(r#""{}", bar::gimme()"#, &["bar"])) .file("bar/Cargo.toml", r#" [project] @@ -210,12 +206,8 @@ fn cargo_compile_with_transitive_dev_deps() { version = "0.5.0" path = "bar" - - [[bin]] - - name = "foo" "#) - .file("src/foo.rs", + .file("src/main.rs", &main_file(r#""{}", bar::gimme()"#, &["bar"])) .file("bar/Cargo.toml", r#" [project] @@ -262,12 +254,10 @@ fn no_rebuild_dependency() { version = "0.5.0" authors = ["wycats@example.com"] - [[bin]] - name = "foo" [dependencies.bar] path = "bar" "#) - .file("src/foo.rs", r#" + .file("src/main.rs", r#" extern crate bar; fn main() { bar::bar() } "#) @@ -294,7 +284,7 @@ fn no_rebuild_dependency() { p.url()))); sleep_ms(1000); - p.change_file("src/foo.rs", r#" + p.change_file("src/main.rs", r#" extern crate bar; fn main() { bar::bar(); } "#); @@ -317,12 +307,10 @@ fn deep_dependencies_trigger_rebuild() { version = "0.5.0" authors = ["wycats@example.com"] - [[bin]] - name = "foo" [dependencies.bar] path = "bar" "#) - .file("src/foo.rs", r#" + .file("src/main.rs", r#" extern crate bar; fn main() { bar::bar() } "#) @@ -412,14 +400,12 @@ fn no_rebuild_two_deps() { version = "0.5.0" authors = ["wycats@example.com"] - [[bin]] - name = "foo" [dependencies.bar] path = "bar" [dependencies.baz] path = "baz" "#) - .file("src/foo.rs", r#" + .file("src/main.rs", r#" extern crate bar; fn main() { bar::bar() } "#) @@ -480,12 +466,8 @@ fn nested_deps_recompile() { version = "0.5.0" path = "src/bar" - - [[bin]] - - name = "foo" "#) - .file("src/foo.rs", + .file("src/main.rs", &main_file(r#""{}", bar::gimme()"#, &["bar"])) .file("src/bar/Cargo.toml", r#" [project] @@ -510,7 +492,7 @@ fn nested_deps_recompile() { p.url()))); sleep_ms(1000); - File::create(&p.root().join("src/foo.rs")).unwrap().write_all(br#" + File::create(&p.root().join("src/main.rs")).unwrap().write_all(br#" fn main() {} "#).unwrap(); @@ -685,12 +667,8 @@ fn path_dep_build_cmd() { version = "0.5.0" path = "bar" - - [[bin]] - - name = "foo" "#) - .file("src/foo.rs", + .file("src/main.rs", &main_file(r#""{}", bar::gimme()"#, &["bar"])) .file("bar/Cargo.toml", r#" [project] diff --git a/tests/test.rs b/tests/test.rs index c37f5a094..686157a32 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -17,7 +17,7 @@ use cargo::util::process; fn cargo_test_simple() { let p = project("foo") .file("Cargo.toml", &basic_bin_manifest("foo")) - .file("src/foo.rs", r#" + .file("src/main.rs", r#" fn hello() -> &'static str { "hello" } @@ -134,7 +134,7 @@ fn cargo_test_overflow_checks() { fn cargo_test_verbose() { let p = project("foo") .file("Cargo.toml", &basic_bin_manifest("foo")) - .file("src/foo.rs", r#" + .file("src/main.rs", r#" fn main() {} #[test] fn test_hello() {} "#); @@ -142,7 +142,7 @@ fn cargo_test_verbose() { assert_that(p.cargo_process("test").arg("-v").arg("hello"), execs().with_status(0).with_stderr(format!("\ [COMPILING] foo v0.5.0 ({url}) -[RUNNING] `rustc [..] src[/]foo.rs [..]` +[RUNNING] `rustc [..] src[/]main.rs [..]` [FINISHED] dev [unoptimized + debuginfo] target(s) in [..] [RUNNING] `[..]target[/]debug[/]deps[/]foo-[..][EXE] hello`", url = p.url())) .with_stdout_contains("test test_hello ... ok")); @@ -182,7 +182,7 @@ fn many_similar_names() { fn cargo_test_failing_test_in_bin() { let p = project("foo") .file("Cargo.toml", &basic_bin_manifest("foo")) - .file("src/foo.rs", r#" + .file("src/main.rs", r#" fn hello() -> &'static str { "hello" } @@ -219,7 +219,7 @@ failures: .with_stdout_contains("[..]`(left == right)`[..]") .with_stdout_contains("[..]left: `\"hello\"`,[..]") .with_stdout_contains("[..]right: `\"nope\"`[..]") - .with_stdout_contains("[..]src[/]foo.rs:12[..]") + .with_stdout_contains("[..]src[/]main.rs:12[..]") .with_stdout_contains("\ failures: test_hello @@ -231,7 +231,7 @@ failures: fn cargo_test_failing_test_in_test() { let p = project("foo") .file("Cargo.toml", &basic_bin_manifest("foo")) - .file("src/foo.rs", r#" + .file("src/main.rs", r#" pub fn main() { println!("hello"); }"#) -- 2.30.2